Add tests to verify the new SubgameRootFinder prototype#611
Add tests to verify the new SubgameRootFinder prototype#611d-kad wants to merge 1 commit intogambitproject:masterfrom
SubgameRootFinder prototype#611Conversation
against the legacy `is_subgame_root`
tturocy
left a comment
There was a problem hiding this comment.
This isn't the way to go about this. We shouldn't add complicated code to the test suite, even if temporarily, because this confounds the test (what if the complicated testing code is itself wrong?)
Instead there is a much more straightforward approach. Write tests for is_subgame_root which simply test whether the node in question should or should not be a subgame root, against an expected value (which expected value we provide, we don't compute it).
We expect the existing is_subgame_root implementation is correct in all cases except one - the one in which there is an absent-minded information set which does not have as one of its members the root of the tree.
We should add a test for such a case as well. That test will fail currently. That is OK. We mark that test with @pytest.mark.xfail (see https://docs.pytest.org/en/stable/how-to/skipping.html#skipping).
Then, when we implement the new algorithm, that test will start passing because it will correct this bug. But that will actually mean the test will be considered by pytest to fail! But that will be good (and at that time we remove the xfail mark from the test and all manner of thing shall be well.
|
Because a fundamentally different approach is needed, closing this one. |
The added tests verify the new
SubgameRootFinderPython prototype against the legacyis_subgame_rootlogic.The new algorithm correctly identifies all legacy subgame roots, and this test suite confirms that any additional roots it finds are exclusively members of absent-minded information sets, which the legacy method was designed to exclude.